Skip to content

Conversation

addisonbeck
Copy link
Contributor

@addisonbeck addisonbeck commented Oct 2, 2025

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-24802

📔 Objective

Enable Nx builds in the browser extension. For example:

# basic commands
npx nx serve browser
npx nx build browser
npx nx test browser
npx nx lint browser

# target the firefix-mv2-dev build 
npx nx serve browser --configuration=firefox-mv2-dev

# targeting a production build (no "dev" suffix)
npx nx build browser --configuration=firefox-mv2

Most complexity here is that we need to support the pre-nx build pipelines for a while, so we have to juggle both configurations. The recent refactor of our webpack config to a webpack config factory was pretty helpful here. I added some optional parameters that come from Nx in Nx builds. Most lines are just changes from relative paths to absolute paths to account for the fact that npm and nx have different working directories.

Once we update CI to use Nx builds exclusively, and the Nx builds have had time to get battle tested, we'll remove the old npm builds and this configuration will get much simpler.

Otherwise, this is a fairly straightforward port of the build commands from extension's package.json into a functioning project.json. It uses the standard executors and follows Nx conventions for this kind of build without much else that is special.

📸 Screenshots

running npx nx test browser

Screenshot 2025-10-10 at 3 26 11 PM

running npx nx lint browser
Screenshot 2025-10-10 at 3 27 14 PM

running npx nx build browser --configuration=safari-commercial
Screenshot 2025-10-10 at 3 30 18 PM

*a running local chrome extension built with npx nx serve browser
Screenshot 2025-10-10 at 3 31 21 PM

⏰ Reminders before review

  • Contributor guidelines followed
  • All formatters and local linters executed and passed
  • Written new unit and / or integration tests where applicable
  • Protected functional changes with optionality (feature flags)
  • Used internationalization (i18n) for all UI strings
  • CI builds passed
  • Communicated to DevOps any deployment requirements
  • Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team

🦮 Reviewer guidelines

  • 👍 (:+1:) or similar for great changes
  • 📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
  • ❓ (:question:) for questions
  • 🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
  • 🎨 (:art:) for suggestions / improvements
  • ❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
  • 🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
  • ⛏ (:pick:) for minor or nitpick changes

This was referenced Oct 2, 2025
Copy link
Contributor

github-actions bot commented Oct 2, 2025

Logo
Checkmarx One – Scan Summary & Details1028d4bf-efd6-49dc-9756-130fec01d5c4

Great job! No new security vulnerabilities introduced in this pull request

Copy link

codecov bot commented Oct 2, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 38.87%. Comparing base (bc48164) to head (6f82e4a).
⚠️ Report is 16 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #16712   +/-   ##
=======================================
  Coverage   38.87%   38.87%           
=======================================
  Files        3420     3420           
  Lines       97313    97313           
  Branches    14625    14625           
=======================================
  Hits        37830    37830           
  Misses      57827    57827           
  Partials     1656     1656           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@addisonbeck addisonbeck force-pushed the add-nx-to-web branch 3 times, most recently from e36012a to 1bca1d8 Compare October 7, 2025 15:08
@addisonbeck addisonbeck force-pushed the add-nx-to-browser branch 5 times, most recently from b76ebc6 to 8c6f822 Compare October 7, 2025 18:51
@addisonbeck addisonbeck changed the base branch from add-nx-to-web to main October 7, 2025 18:51
Comment on lines +1 to +5
{
"extends": "./tsconfig.json",
"include": ["src", "../../libs/common/src/autofill/constants"],
"exclude": ["**/*.stories.*", "**/*.spec.ts"]
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this PR in particular I also added tsconfig.build.json files that exclude stories from builds. Without this nx builds were failing due to some legitimate errors in the components project that aren't revealed by other builds. I don't condone this pattern in general, but didn't want to get distracted and decided to follow the pattern already established in the web vault with its tsconfig.build.json files already committed here and here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@addisonbeck We should create a follow-up ticket or contact the team responsible for those stories so that they can fix it and we can remove the this workaround

Copy link

@addisonbeck addisonbeck marked this pull request as ready for review October 10, 2025 19:33
@addisonbeck addisonbeck requested a review from a team as a code owner October 10, 2025 19:33
@addisonbeck addisonbeck merged commit 0dd09ca into main Oct 14, 2025
62 checks passed
@addisonbeck addisonbeck deleted the add-nx-to-browser branch October 14, 2025 10:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants